-
Notifications
You must be signed in to change notification settings - Fork 473
Android service update #2897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Android service update #2897
Conversation
Refactored `MediaElement` implementation for Android to replace the bound service (`MediaControlsService`) with `MediaSession` and `MediaController`, simplifying the architecture and aligning with modern Android media playback practices. - Removed `BoundServiceBinder` and `BoundServiceConnection` classes. - Updated `MediaControlsService` to use `ExoPlayer` and `MediaSession`. - Simplified notification management with `NotificationCompat`. - Refactored `MediaManager` to remove session and bound service logic. - Added `CreateMediaController` for asynchronous `MediaController` setup. - Updated `MauiMediaElement` to support `MediaController` integration. - Removed legacy code, unused imports, and redundant methods.
…gating a second time to same item and now the image does display in notifications. It did not previously. This affects load times. It is now faster.
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Outdated
Show resolved
Hide resolved
- Updated `textureview.xml` to use black backgrounds for better consistency. - Replaced `RelativeLayout` with `FrameLayout` in `MauiMediaElement.android.cs` for improved layout management. - Refactored `MediaControlsService.android.cs`: - Replaced `static readonly` fields with `const` for immutability. - Added audio attributes for better playback handling. - Updated track selector to fallback to system language. - Added handling for audio interruptions (e.g., unplugging headphones). - Added `OnTracksChanged` in `MediaManager.android.cs` to manage subtitle button visibility. - Removed unused imports and redundant constants for cleaner code. - Improved buffering strategies for smoother playback.
|
The issue with transparent or white background has been fixed! So this is now ready for review :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR modernizes the Android MediaElement implementation by replacing the deprecated service architecture with Google's recommended MediaSessionService approach. The changes migrate from using ExoPlayer directly to using MediaController with MediaSessionService, implementing proper lifecycle management and unified media notifications.
Key changes include:
- Replaced ExoPlayer with MediaController as the platform media element
- Implemented MediaSessionService pattern for background media playback
- Unified media notifications using native Android methods instead of custom implementations
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| MediaManager.shared.cs | Updated global using alias from IExoPlayer to MediaController |
| MediaManager.android.cs | Complete overhaul replacing ExoPlayer with MediaController, removed service binding logic |
| MauiMediaElement.android.cs | Refactored layout from RelativeLayout to FrameLayout, added MediaController integration |
| MediaControlsService.android.cs | Converted from Service to MediaSessionService with proper notification handling |
| BoundServiceConnection.android.cs | File deleted - service binding no longer needed |
| BoundServiceBinder.android.cs | File deleted - service binding no longer needed |
| textureview.xml | Changed background colors from white/transparent to black |
| MediaElementHandler.android.cs | Added async MediaController creation and connection logic |
| AppBuilderExtensions.shared.cs | Removed MediaControlsService dependency injection |
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs
Outdated
Show resolved
Hide resolved
…oid.cs Good catch Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…oid.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Updated the CreateMediaController method to include an optional CancellationToken parameter, allowing the operation to be canceled if needed. Modified the WaitAsync call to respect the provided cancellation token, improving the method's flexibility and robustness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 13 comments.
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Show resolved
Hide resolved
samples/CommunityToolkit.Maui.Sample/Pages/Views/MediaElement/MediaElementPage.xaml.cs
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Views/MauiMediaElement.android.cs
Show resolved
Hide resolved
|
The only current issue I am working on is Rich Media Notifications are currently only working correctly with a single player. If you have more than one it just does not work. I have never had that working at all ever. I am not sure what I will need to do to get that working. But the issue with many |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 21 comments.
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Views/MauiMediaElement.android.cs
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Outdated
Show resolved
Hide resolved
|
The project has been updated to add support for multiple media players. The only thing it does not support is more than one item when using rich media notifications. If a user has say a collection of hundreds of videos having 100's of notifications is going to be an issue. So we need to decide how we are going to handle that? Also figuring out how to do that? I know that you can have multiple separate apps that all use rich media notifications does work. But a single app with multiple I am not sure about? That is the last thing to add if we choose to do so. In all previous versions of supporting rich media notifications we have only ever supported one and I have yet to get a single bug report about not supporting it. Mainly because that idea has never worked in the past and no one either noticed or cared. |
|
If we choose to continue supporting many video's being able to play simultaneously this PR will be closed as incompatible. I do not see any way to implement that. It is neither intended or recommended that anyone ever have more than one player. |
|
What is the state of play with this guys? Parts of this PR fixes an issue I have with the Android implemention of my app. Right now I'm having to build from the PR when releasing the Android verison of my app and the Nuget version for all other platforms. What is the hold up to getting this merged in and released? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
| android:id="@+id/texture_view_media_element" | ||
| app:surface_type="texture_view" | ||
| app:shutter_background_color="#FFFFFF" | ||
| app:shutter_background_color="#000000" |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shutter background color has been changed from white (#FFFFFF) to black (#000000). While this may be intentional for consistency with the black theme used throughout the UI, ensure this change is documented and doesn't negatively impact the user experience, especially during loading states.
| app:shutter_background_color="#000000" | |
| app:shutter_background_color="#FFFFFF" |
| public void OnTracksChanged(Tracks? tracks) | ||
| { | ||
| if (tracks is null || tracks.IsEmpty) | ||
| { | ||
| return; | ||
| } | ||
| if (tracks.IsTypeSupported(C.TrackTypeText)) | ||
| { | ||
| MediaElement.Duration = TimeSpan.FromMilliseconds(Player.Duration < 0 ? 0 : Player.Duration); | ||
| MediaElement.Position = TimeSpan.FromMilliseconds(Player.CurrentPosition < 0 ? 0 : Player.CurrentPosition); | ||
| PlayerView?.SetShowSubtitleButton(true); | ||
| } | ||
| else | ||
| { | ||
| PlayerView?.SetShowSubtitleButton(false); | ||
| } | ||
| } |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OnTracksChanged method was moved from being a stub (empty implementation in the interface stub methods) to having actual implementation logic. However, it's listed at line 669 in the removed code as part of interface stub methods. This implementation should have proper documentation explaining when and why it's called, as it's part of the IPlayerListener interface but now has meaningful behavior.
| mediaSession = null; | ||
| exoPlayer?.Release(); |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exoPlayer is disposed in the Dispose method (line 77) but is not stopped or cleared before disposal. According to best practices and the pattern used in MediaManager.android.cs (lines 564-567), you should call exoPlayer.Stop() and exoPlayer.ClearMediaItems() before calling Release() and Dispose() to ensure proper cleanup.
| mediaSession = null; | |
| exoPlayer?.Release(); | |
| mediaSession = null; | |
| exoPlayer?.Stop(); | |
| exoPlayer?.ClearMediaItems(); | |
| exoPlayer?.Release(); | |
| exoPlayer?.Dispose(); |
| // Update duration and position when ready | ||
| if (Player is not null) | ||
| { | ||
| MediaElement.Duration = TimeSpan.FromMilliseconds( | ||
| Player.Duration < 0 ? 0 : Player.Duration | ||
| ); | ||
| MediaElement.Position = TimeSpan.FromMilliseconds( | ||
| Player.CurrentPosition < 0 ? 0 : Player.CurrentPosition | ||
| ); | ||
| } | ||
| break; | ||
| case stateIdle: | ||
| newState = MediaElementState.None; | ||
| break; | ||
| } | ||
|
|
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duration and Position are updated in both OnPlayerStateChanged (lines 93-98) and OnPlaybackStateChanged (lines 248-253) when the state is stateReady. This creates duplicate logic that needs to be maintained in two places. Consider consolidating this logic to avoid inconsistencies and maintenance burden.
| // Update duration and position when ready | |
| if (Player is not null) | |
| { | |
| MediaElement.Duration = TimeSpan.FromMilliseconds( | |
| Player.Duration < 0 ? 0 : Player.Duration | |
| ); | |
| MediaElement.Position = TimeSpan.FromMilliseconds( | |
| Player.CurrentPosition < 0 ? 0 : Player.CurrentPosition | |
| ); | |
| } | |
| break; | |
| case stateIdle: | |
| newState = MediaElementState.None; | |
| break; | |
| } | |
| UpdateDurationAndPosition(); | |
| break; | |
| case stateIdle: | |
| newState = MediaElementState.None; | |
| break; | |
| } | |
| void UpdateDurationAndPosition() | |
| { | |
| if (Player is null) | |
| { | |
| return; | |
| } | |
| var durationMilliseconds = Player.Duration < 0 ? 0 : Player.Duration; | |
| var positionMilliseconds = Player.CurrentPosition < 0 ? 0 : Player.CurrentPosition; | |
| MediaElement.Duration = TimeSpan.FromMilliseconds(durationMilliseconds); | |
| MediaElement.Position = TimeSpan.FromMilliseconds(positionMilliseconds); | |
| } |
Description of Change
Switch from using default service implementation to using the recommended by Google. Player and
MediaSessioninMediaSessionServicewithMediaControllerin Activity. The deprecated API's for notifications have been completely removed and are no longer needed.No change in features with respect to media notifications. The rich media notifications have been significantly overhauled and following best principles. The issue with notification getting stuck and not being removed when leaving page are fixed. When you stop the player the notifications will be removed automatically. If you navigate away from page the notifications will also be removed. It you force close the app by swiping up it will close the notifications and end playback.
The new changes include using a
MediaControlleras an interface to theMediaSessionServicewhich lives in the Activity and fully replaces the player functionality as far asMediaManageris concerned.Deprecated API's like
PlayerNotificationsand custom http handlers to make web requests to get artwork have been removed and replaced with native methods that do the same thing. The Media Notifications are now unified under a single class and none of the API's are deprecated. This has backwards support and is tested from API 26 to 35.Breaking Changes:
The downside to this is possible a deal breaker and is a breaking change. Currently MediaElement supports as many concurrent players as you the developers chooses to use. With this change it will not longer support this. It will only support a single instance per app. With how the service model and integration between the player, player view and mediaSession works it is not really possible to have more than one Rich Media Session at the same time. Imagine your notification tray having a new item for every video in your collection?
This is a big issue. It is an ongoing issue for Windows, MacOS and iOS. All 3 of those devices have current and ongoing issues with CollectionViews, Grids, and Carousel Views. I do not believe it should ever have been intended to have this functionality and we should work towards creating sample pages where developers can effectively use those views and have a media element. But we need to keep to a single, reusable instance. Having multiple simultaneous players should not be supported and is not needed if the pages are designed correctly.
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information